Swift SDK for WASM using the run destination #966
Conversation
The client can provide a Swift SDK manifest path along with a triple in the run destination to synthesize a Swift Build SDK. This serves as a fallback for cases where Swift Build cannot match an SDK for it. Note that in this first implementation the platform is hard coded to webassembly, and so only SDK's that align with WASM, such as static linking of compiler runtime pieces, and some Unix semantics will work.
…c instead of the base ld xcspec
Sources/SWBCore/SDKRegistry.swift
Outdated
| "SWIFT_RESOURCE_DIR": .plString(swiftResourceDir.str), // Resource dir for compiling Swift | ||
| "CLANG_RESOURCE_DIR": .plString(clangResourceDir.str), // Resource dir for linking C/C++/Obj-C | ||
| "SDKROOT": .plString(sysroot.str), | ||
| "OTHER_LDFLAGS": .plArray(["$(OTHER_SWIFT_FLAGS)"]), // The extra swift compiler settings in JSON are intended to go to the linker driver too |
There was a problem hiding this comment.
If we need to include swift flags here, we should use extraSwiftCompilerSettings instead of $(OTHER_SWIFT_FLAGS), because $(OTHER_SWIFT_FLAGS) may include other project/command line flags which are only intended for the compiler
Add the missing sdk parameter to swiftc linker driver in UnixLD.xcpsec
…izes in producing one from run destination information
jakepetroules
left a comment
jakepetroules
left a comment
There was a problem hiding this comment.
Looking good! :)
Left another round of feedback.
| { | ||
| Name = CLANG_SDKROOT_LINKER_INPUT; | ||
| Type = Path; | ||
| DefaultValue = "$(SDKROOT)"; |
There was a problem hiding this comment.
I think this one needs to be $(SYSROOT:default=$(SDKROOT))
| { | ||
| Name = SWIFTC_SDKROOT_LINKER_INPUT; | ||
| Type = Path; | ||
| DefaultValue = "$(SYSROOT:default=$(SDKROOT))"; |
There was a problem hiding this comment.
This doesn't seem right -- based on discussions around how the various platform-targeting flags would work, the consensus was:
- When targeting platforms where Swift SDK and sysroot are the same (Apple platforms), use
-sdkonly - When targeting Windows, use
-sdkonly since the sysroot concept doesn't exist - For all others, use both
-sdkand-sysroot
Wasm falls into bullet 3, so I think the sdk flag needs to be based on $(SDKROOT), while the $(SYSROOT) var would be used to pass -sysroot. That means you should be able to remove this override and use the one from the base Ld.xcspec.
There was a problem hiding this comment.
The UnixLd.xcspec is getting in the way between Ld.xcspec and this WasmLd.xcspec. If I remove this then it will prevent it from adding the required "-sdk" argument to the linker driver.
There was a problem hiding this comment.
The point is that the DefaultValue here is not correct -- SYSROOT should not influence the -sdk flag. Only SDKROOT should. Meaning you need this change:
| DefaultValue = "$(SYSROOT:default=$(SDKROOT))"; | |
| DefaultValue = "$(SDKROOT)"; |
and which consequently makes this equivalent to the base definition.
There was a problem hiding this comment.
Thanks, I've adjusted the WasmLD.xcspec to consider only the SDKROOT, and not the SYSROOT.
There was a problem hiding this comment.
You should still remove the SWIFTC_SDKROOT_LINKER_INPUT definition from WasmLd.xcspec though, since it's now identical to the base definition in Ld.xcspec and is therefore a no-op.
There was a problem hiding this comment.
There's still the interim UnixLd.xcspec that interferes, so this is needed to override that until it is fixed.
There was a problem hiding this comment.
Ahh, I missed that. That is certainly a bit confusing that the Ld.xcspec setting is passing -sdk while the same setting in UnixLd.xcspec is passing -sysroot. Something there seems not quite right; I think we need to have two settings in UnixLd.xcspec, one passing -sdk and one passing -sysroot, since that model applies to all Unix-like platforms (except Apple).
Fix the SDKROOT default value
…t to determine platform from triple
owenv
left a comment
owenv
left a comment
There was a problem hiding this comment.
This is looking good to me, I think it's pretty much good as a first step to land once we get some basic tests in place. Left one minor comment/question
jakepetroules
left a comment
jakepetroules
left a comment
There was a problem hiding this comment.
Looking pretty good, mostly some style feedback plus a small blocking issue around the Linux triples.
|
@swift-ci please test |
|
The only remaining feedback I have so far are the two unresolved comments related to SYSROOT/SDKROOT/-sdk/-sysroot. |
3 participants
The client can provide a Swift SDK manifest path along with a triple in the run destination to synthesize a Swift Build SDK. This serves as a fallback for cases where Swift Build cannot match an SDK for it.
Note that in this first implementation the platform is hard coded to webassembly, and so only SDK's that align with WASM, such as static linking of compiler runtime pieces, and some Unix semantics will work.